<!DOCTYPE html>
<!--
     SPDX-License-Identifier: CC-BY-SA-4.0
     SPDX-FileCopyrightText: 2020 seL4 Project a Series of LF Projects, LLC.
-->
<!-- Page last generated 2025-02-20 03:16:15 +0000 -->
<html lang="en">
  <head>
    <meta charset="utf-8">
    <meta http-equiv="X-UA-Compatible" content="IE=edge">
    <meta name="viewport" content="width=device-width, initial-scale=1">
    <title>Pull Request Conventions | seL4 docs</title>

    <!-- Our stylesheet and theme stylesheet.  Contains bootstrap. -->
    <link rel="stylesheet" href="/assets/css/style.css" type="text/css">
    <!-- Font awesome -->
    <link href="https://use.fontawesome.com/releases/v5.0.8/css/all.css" rel="stylesheet">
    <link href="https://fonts.googleapis.com/css2?family=Roboto&display=swap" rel="stylesheet">
    <!-- Pygments syntax highlighting  -->
    <link rel="stylesheet" href="/assets/css/highlighting/trac.css" type="text/css">
    <link rel="icon" type="image/x-icon" href="/assets/favicon.ico"><script defer data-domain="docs.sel4.systems"
	    src="https://analytics.sel4.systems/js/script.js"></script></head>

  <body class="container-fluid">

    



<header>
  <ul class="row menu">
    <li class="col-xs-12 col-md-2" >
            <a href="https://sel4.systems" class="skip-icon">
              <img class="img-responsive" src="/assets/logo-text-white.svg" alt="seL4 logo" />
            </a>
    </li>
    <li class="col-xs-12 col-md-10 menu">
      <nav aria-label="Banner links">
        <h2><a href="/Resources" />Resources</h2>
        <h2><a href="/processes" />Contributing</a></h2>
        <h2><a href="/projects" />Projects</h2>
        <h2><a href="/Tutorials" />Tutorials</h2>
        <iframe title="DuckDuckGo search bar" src="https://duckduckgo.com/search.html?site=docs.sel4.systems&prefill=Search%20sel4.systems" style="overflow:hidden;margin-bottom:10px; padding:0;height:40px;float:right;border-width: 0px"></iframe>
      </nav>
    </li>
  </ul>
  <div class="clear"></div>
  
<div class="breadcrumbs bootstrap hidden-sm-down">
  <nav class="sel-breadcrumb" aria-label="Breadcrumb" >
    <ol class=" list-unstyled" vocab="http://schema.org/" typeof="BreadcrumbList">
      
      
        

        

        <li class="breadcrumb-item" property="itemListElement" typeof="ListItem">
            <a property="item" typeof="WebPage" href="/">
              <span property="name"><b>seL4 Docs</b></span>
            </a>
            <meta property="position" content="1" />
        </li>
      
        

        

        <li class="breadcrumb-item" property="itemListElement" typeof="ListItem">
            <a property="item" typeof="WebPage" href="/processes/">
              <span property="name"><b>Processes</b></span>
            </a>
            <meta property="position" content="2" />
        </li>
      
        

        
          <li class="breadcrumb-item" property="itemListElement" typeof="ListItem">
            <span property="name">Pull Request Conventions</span>
            <meta property="position" content="3" /></li>
          
    </ol>
  </nav>
  <nav class="sel-version" aria-label="Current Versions">
    <ol class="list-unstyled">
      <li class="list-unstyled text-right" style="margin-left:auto; padding:0rem 0rem;">
        Current versions:</li>
      <li class="list-unstyled text-right">
      <a href="/releases/sel4/13.0.0"><b>seL4-13.0.0</b></a></li>
      <li class="list-unstyled text-right">
      <a href="/releases/microkit/1.4.1"><b>microkit-1.4.1</b></a></li>
      <li class="list-unstyled text-right">
      <a href="/releases/camkes/camkes-3.11.0"><b>camkes-3.11.0</b></a></li>
      <li class="list-unstyled text-right">
      <a href="/releases/capdl/0.3.0"><b>capDL-0.3.0</b></a></li>
      </ol>
  </nav>
  <div class='clear'></div>
</div>


</header>

    <main>
      <div class="row">
  <div class="hidden-xs col-sm-4 col-md-3 col-lg-2">
    


<div class="sidebar">






      <ul class="nav nav-sidebar">
  
        <li class="">
          <a class="" href="/processes/conduct.html">
            Code of Conduct
          </a>
        </li>
  
        <li class="">
          <a class="" href="/processes/contributing.html">
            Contributing
          </a>
        </li>
  
        <li class="">
          <a class="" href="/processes/licenses.html">
            Licensing
          </a>
        </li>
  
        <li class="">
          <a class="" href="/processes/style-guide.html">
            Style Guide
          </a>
        </li>
  
        <li class="">
          <a class="" href="/processes/git-conventions.html">
            Git Conventions
          </a>
        </li>
  
        <li class="active">
          <a class="" href="/processes/code-review.html">
            Pull requests
          </a>
        </li>
  
        <li class="">
          <a class="" href="/processes/release-process.html">
            Release process
          </a>
        </li>
  
        <li class="">
          <a class="" href="/processes/rfc-process.html">
            RFC process
          </a>
        </li>
  
        <li class="">
          <a class="" href="/processes/test-status.html">
            Test Status
          </a>
        </li>
  
        <li class="">
          <a class="" href="/processes/websites.html">
            Websites
          </a>
        </li>
  
        <li class="">
          <a class="" href="/processes/emails.html">
            Emails
          </a>
        </li>
  
        <li class="">
          <a class="" href="/MaintainedRepositories.html">
            Maintained Repositories
          </a>
        </li>
  
      </ul>











</div>

  </div>
  <div class="col-sm-8 col-md-9 col-lg-8 main">
    
    <div class="content">
      <h1 id="pull-request-conventions">Pull Request Conventions</h1>

<p>This is a guide for conducting effective and efficient pull requests in our code projects and
applies to all github sources.</p>

<p>In general, a pull request (PR) should communicate what the change is, and why it’s necessary.</p>

<h2 id="creation">Creation</h2>

<p>Pull requests should be created from personal forks. We follow a <a href="https://www.atlassian.com/git/tutorials/comparing-workflows/forking-workflow">fork-rebase
workflow</a>.</p>

<h3 id="title">Title</h3>

<ul>
  <li>For a single commit, the title is the subject line of the commit message.</li>
  <li>Otherwise, the title should summarise the set of commits.</li>
</ul>

<h3 id="description">Description</h3>

<ul>
  <li>Must state the <em>why</em> and the <em>how</em> for the change.
    <ul>
      <li>Usually this is the body of your commit message.</li>
    </ul>
  </li>
  <li>Must explain the purpose of the PR, e.g.:
    <ul>
      <li>feedback for an initial implementation,</li>
      <li>request for comment,</li>
      <li>ready to merge.</li>
    </ul>
  </li>
  <li>Explain any context:
    <ul>
      <li>is it part of a greater set of changes?</li>
      <li>are any concurrent PRs (in other repositories) dependent on this PR?</li>
    </ul>
  </li>
  <li>State what testing has been performed:
    <ul>
      <li>Run sel4test and for which platforms.</li>
      <li>If a new major feature has been added, corresponding tests must be added to <code class="language-plaintext highlighter-rouge">sel4test</code>.</li>
    </ul>
  </li>
</ul>

<h3 id="reviewers">Reviewers</h3>

<ul>
  <li>The <a href="https://sel4.systems/Foundation/TSC">TSC</a> of the seL4 foundation will delegate reviewers to approve. Anyone can help to review a pull request.</li>
  <li>If you want a particular person to review, please tag them.</li>
  <li>If there hasn’t been any activity after a couple of days, feel free to bump the post.</li>
  <li>Pull requests require at least one approving review for merge,
and usually should aim for 2 reviews on each non-trivial pull request.</li>
</ul>

<h3 id="commits">Commits</h3>

<ul>
  <li>Commit history is part of the review
    <ul>
      <li>A good commit history assists reviewers in understanding the change</li>
      <li>Please see the <a href="/GitConventions">Git conventions</a>.</li>
    </ul>
  </li>
  <li>Good reviews are small reviews. Large PRs should only be created if necessary.</li>
</ul>

<h3 id="tests">Tests</h3>

<ul>
  <li>The foundation repositories require the following tests to pass
before a pull request can be merged:
    <ul>
      <li>code style
        <ul>
          <li>these may vary per repository and language, but default should
standardise on those in the repository
<a href="https://github.com/seL4/seL4_tools/tree/master/misc">seL4/seL4_tools</a></li>
        </ul>
      </li>
      <li>developer certificate of origin (<a href="contributing.html">DCO</a>)</li>
      <li>checks for SPDX license tagging (using the <a href="https://reuse.software">REUSE tool</a>)</li>
      <li>any applicable regression tests:
        <ul>
          <li>these vary per repository</li>
          <li>for seL4 itself, they must include:
            <ul>
              <li>compile test</li>
              <li>hardware and/or simulator runs</li>
              <li>the proofs</li>
            </ul>
          </li>
          <li>for verification target repositories (currently mainly seL4):
            <ul>
              <li>a pull request can only be merged on the master branch if either
the corresponding proof is updated or if there is no proof impact.</li>
              <li>there is no proof impact if:
                <ul>
                  <li>the preprocessed source for verified code has not changed
(this is tested by the “preprocess” check on GitHub), or</li>
                  <li>the proof still works unchanged despite the code change
(please ping the <code class="language-plaintext highlighter-rouge">@verification</code> team on the GitHub seL4 org when
the “preprocess” check fails and you think the proof might still
work).</li>
                </ul>
              </li>
              <li>for proof updates:
                <ul>
                  <li>submit a pull request to the <a href="https://github.com/seL4/l4v"><code class="language-plaintext highlighter-rouge">l4v</code></a>
repository together with the
pull request for the <a href="https://github.com/seL4/seL4"><code class="language-plaintext highlighter-rouge">seL4</code></a>
repository, or</li>
                  <li>ping the <code class="language-plaintext highlighter-rouge">@verification</code> team on the GitHub seL4 org for help in
updating the proofs, or</li>
                  <li>talk to the <a href="https://sel4.systems/Foundation/">seL4 Foundation</a> about finding funding
and/or volunteers for the proofs updates if it is a bigger project.</li>
                </ul>
              </li>
            </ul>
          </li>
        </ul>
      </li>
    </ul>
  </li>
  <li>Exceptions are possible by approval of someone in the <a href="roles.html">Committer</a> role</li>
</ul>

<h2 id="during-a-pr">During a PR</h2>

<ul>
  <li>Always abide by the <a href="/Conduct">seL4 Code of Conduct</a>.</li>
</ul>

<h3 id="for-reviewers">For Reviewers</h3>

<ul>
  <li>Take into account the context stated by the author.</li>
  <li>Review commits and commit messages as well as code</li>
  <li>Request that the above guide be followed if it is not.</li>
  <li>Provide constructive feedback.
    <ul>
      <li>see the resources below.</li>
    </ul>
  </li>
  <li>Remember to comment on good things.</li>
</ul>

<h3 id="for-authors">For Authors</h3>

<ul>
  <li>Apply changes due to feedback from reviewers as additional commits, and squash them once the PR is
ready to merge.</li>
  <li>Please attempt to only push changes to the PR branch once it is ready for re-review.</li>
  <li>Please communicate any changes you make during the review process.</li>
  <li>Apart from editing history, or fixing trivial issues, do not push changes to a PR once it has been
approved.</li>
</ul>

<h2 id="merging">Merging</h2>

<ul>
  <li>
    <p>Anyone in the <a href="roles.html">Committer</a> role can merge pull requests after they
satisfy the required tests and approvals.</p>
  </li>
  <li>
    <p>Currently, for many repositories in the seL4 GitHub org, the
Trustworthy Systems (TS) group still provides continuous integration
(CI) infrastructure on their internal servers. For these
repositories, someone from TS will merge the pull request on that
internal infrastructure, and it will be pushed out to GitHub
automatically by the CI pipeline. Some of these tests, especially
those involving proofs may run for multiple hours, some more than
24h, so it might take some time for the merge to become visible.</p>

    <p>You can recognise these repositories by the fact that they require 6
(instead of 1) approving reviews for pull requests – this is merely a
mechanism to prevent accidental merges.</p>

    <p>The seL4 foundation is working on making this CI infrastructure more
accessible directly on GitHub to avoid this additional step, but it
will require some time and resources to do so. If you’re interested
in helping with this, please email the <a href="https://sel4.systems/Foundation/TSC/">TSC chair</a>.</p>
  </li>
  <li>
    <p>As long as the CI infrastructure is hosted by TS, some pull requests
will continue to be handled directly internally on the TS group’s
infrastructure. The TS group must abide by the same rules for these
as outlined above.</p>
  </li>
</ul>

<h2 id="resources">Resources</h2>

<ul>
  <li><a href="http://fishbowl.pastiche.org/2009/10/20/the_principle_of_charity_2/">The principle of charity</a></li>
  <li>Code review for humans <a href="https://mtlynch.io/human-code-reviews-1/">part 1</a> and <a href="https://mtlynch.io/human-code-reviews-2/">part
2</a></li>
</ul>


    </div>
  </div>
</div>

    </main>
    


<footer class="site-footer">

  <h2 class="footer-heading">seL4 docs</h2>

  <div class="footer-col-wrapper">

    <div class="col-md-2">
      



<ul class="social-media-list">
  <li><a href="https://github.com/sel4"><i class="fab fa-github"></i> <span class="username">sel4</span></a></li><li><a href="https://github.com/sel4proj"><i class="fab fa-github"></i> <span class="username">sel4proj</span></a></li>
</ul>

    </div>

    <div class="col-md-8">
      <ul class="list-unstyled">
        <li>
          This site is for displaying seL4 related documentation.  Pull requests are welcome.
        </li>
        
          <li>
            Site last updated: Fri Feb 7 10:17:38 2025 +1100 ee78c8857c
          </li>
          <li>
                Page last updated: Wed Jul 28 11:02:39 2021 +1000 4e9d4ab1f5
          </li>
        
      </ul>
    </div>
    <div class="col-md-2">
<a href="https://github.com/seL4/docs/blob/master/processes/code-review.md">View page on GitHub</a>
      <br />
      <a href="https://github.com/seL4/docs/edit/master/processes/code-review.md">Edit page on GitHub</a>
      <br />
      <a href="/sitemap">Sitemap</a>
    </div>

  </div>

</footer>

  </body>
</html>
